-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable CI tests for discovery node #1047
Conversation
@@ -148,13 +150,53 @@ jobs: | |||
env: | |||
IMAGE_NAME: ${{ github.repository }} | |||
REF_NAME: ${{ github.ref_name }} | |||
DEVICE_ID: ${{'AHU-1'}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No ${{
run: | | ||
docker run --net udminet --name registrar -v $(realpath site_model):/root/site_model \ | ||
ghcr.io/$IMAGE_NAME:validator-$REF_NAME bin/registrar site_model/cloud_iot_config.json | ||
- name: Start discoverynode | ||
run: | | ||
export IMAGE_TAG=ghcr.io/$IMAGE_NAME:misc-$REF_NAME | ||
# This currently fails (no config), but just check that it actually runs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove comment
@@ -11,8 +11,8 @@ ROOT_DIR=$(realpath $(dirname $0)/..) | |||
BASE_CONFIG=$(realpath $ROOT_DIR/etc/base_config.json) | |||
TMP_CONFIG="/tmp/discoverynode_config.json" | |||
|
|||
if [[ $# -ne 3 ]]; then | |||
error Usage: $0 SITE_MODEL TARGET DEVICE_ID | |||
if (( $# -le 4 )); then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be -le 2 since the [OPTIONS] bit is optional? Or -lt 3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's three required options:
- site model
- target
- device ID
misc/discoverynode/bin/run
Outdated
|
||
cat $TMP_CONFIG | jq -r "$substitutions" | sponge $TMP_CONFIG | ||
|
||
for option in "$@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
convention is to have "; do" at the end, rather than "do" on a new line
Yes, and if so, you would have (( 3 -le 4 )); then error?
…On Tue, Dec 10, 2024 at 1:43 AM Noureddine ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In misc/discoverynode/bin/run
<#1047 (comment)>:
> @@ -11,8 +11,8 @@ ROOT_DIR=$(realpath $(dirname $0)/..)
BASE_CONFIG=$(realpath $ROOT_DIR/etc/base_config.json)
TMP_CONFIG="/tmp/discoverynode_config.json"
-if [[ $# -ne 3 ]]; then
- error Usage: $0 SITE_MODEL TARGET DEVICE_ID
+if (( $# -le 4 )); then
There's three required options:
- site model
- target
- device ID
—
Reply to this email directly, view it on GitHub
<#1047 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIEPDZTVI7WHKBA2HOJPSL2E2ZSXAVCNFSM6AAAAABTJUVYCKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIOJRGY4TAOBXGQ>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
what's the status of this? Submit? Draft? Close? |
in progress, there's some peculiar auth bug in the non integration test environment so I'm just going to split the PR so the working integration test can be merged in |
No description provided.